-
Notifications
You must be signed in to change notification settings - Fork 898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add policy checking for request_host_scan. #14427
Conversation
@lfu Cannot apply the following label because they are not recognized: drag/yes |
Some comments on commit lfu@e69e061 spec/models/host_spec.rb
|
Checked commit lfu@e69e061 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
_log.warn("Error raising request scan event for #{log_target}: #{err.message}") | ||
return | ||
end | ||
check_policy_prevent(:request_host_scan, :scan_queue, userid, options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lfu We already have a scan_from_queue
method on the Host
model. Can the logic here be reworked so the check_policy_prevent
calls that method when the scan is not being stopped by policy? Otherwise we are calling the scan_queue
method just to put the scan on the queue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gmcculloug Should not Host Scan go through the queue waiting for its turn?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The host scan does need to be queued and we already do that with the existing scan_from_queue
method.
My question was asking if we could get away without having to call the new scan_queue
which then queues the actual scan. Might be too much effort to do this using check_policy_prevent
but it seemed worth looking into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gmcculloug scan_from_queue
does not put the host scan on to the queue. It actually does the scan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lfu and I discussed my question above and agreed to leave the code as-is for now.
@miq-bot add_label darga/yes |
@lfu unrecognized command 'add_lable', ignoring... Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone |
@lfu Is there a BZ for this? Can you please create one if it doesn't exist? |
@simaishi BZ ticket was added. |
Add policy checking for request_host_scan. (cherry picked from commit 5e9e1c7) https://bugzilla.redhat.com/show_bug.cgi?id=1437922
Fine backport details:
|
Add policy checking for request_host_scan. (cherry picked from commit 5e9e1c7) https://bugzilla.redhat.com/show_bug.cgi?id=1437925
Euwe backport details:
|
All events are sent to automate for processing since Event Switchboard PR.
Since then the policy checking for prevent action has changed from a sync process to an async process.
https://bugzilla.redhat.com/show_bug.cgi?id=1437910
@miq-bot assign @gmcculloug
@miq-bot add_label bug, control, darga/yes, euwe/yes